Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

revert: "fix: pass original fetch args along" #1371

Merged
merged 1 commit into from
Aug 20, 2024
Merged

revert: "fix: pass original fetch args along" #1371

merged 1 commit into from
Aug 20, 2024

Conversation

pauldambra
Copy link
Member

Reverts #1351

we've seen multiple reports of fetch wrapping causing errors, it would be amazing if it wasn't this change causing it.

it's safe to revert if it's not this change and important to revert if it is...

@vercel Vercel
Copy link

vercel bot commented Aug 20, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Aug 20, 2024 7:35am

@pauldambra pauldambra added the bump patch Bump patch version when this PR gets merged label Aug 20, 2024
@pauldambra pauldambra changed the title revert "fix: pass original fetch args along" revert: "fix: pass original fetch args along" Aug 20, 2024
@github-actions GitHub Actions
Copy link

Size Change: -6 B (0%)

Total Size: 1.17 MB

Filename Size Change
dist/array.full.js 333 kB -2 B (0%)
dist/recorder-v2.js 110 kB -2 B (0%)
dist/recorder.js 110 kB -2 B (0%)
ℹ️ View Unchanged
Filename Size
dist/array.js 154 kB
dist/exception-autocapture.js 10.4 kB
dist/main.js 155 kB
dist/module.js 154 kB
dist/surveys-preview.js 59.8 kB
dist/surveys.js 66 kB
dist/tracing-headers.js 8.26 kB
dist/web-vitals.js 5.79 kB

compressed-size-action

@pauldambra pauldambra mentioned this pull request Aug 20, 2024
Closed
@benjackwhite benjackwhite merged commit a673570 into main Aug 20, 2024
13 of 14 checks passed
@benjackwhite benjackwhite deleted the revert-1351-fix/pass-original-along branch August 20, 2024 08:15
@ricardobeat
Copy link

The same error has been fixed in dd-trace a while ago by calling fetch with the original input Request object: https://github.com/DataDog/dd-trace-js/pull/4258/files

Phanatic pushed a commit that referenced this pull request Aug 20, 2024
@pauldambra @Phanatic
Revert "fix: pass original fetch args along (#1351)" (#1371)
2012043
@pauldambra
Copy link
Member Author

@ricardobeat interestingly that's a fix in DD for a different issue. their thread mentions it is difficult to test for but I'll see if i can poke about and recreate for us since we might be open to the same error as well (although haven't had it reported)

@xiezhifeng xiezhifeng mentioned this pull request Sep 24, 2024
Open
@Bad3r Bad3r mentioned this pull request Sep 24, 2024
Open
@ricardobeat
Copy link

@pauldambra the core issue is the same, wrapping fetch incorrectly. I meant to point to a potentially better solution vs the one at 24979cb which has been reverted, preserving a Request object if received is more reliable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neilkakkar neilkakkar neilkakkar approved these changes

@daibhin daibhin daibhin approved these changes

Assignees
No one assigned
Labels
bump patch Bump patch version when this PR gets merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pauldambra @ricardobeat @daibhin @neilkakkar @benjackwhite